CBST2-04: Update JWT secrets on reload and revoke module endpoint #295
Conversation
crates/signer/src/service.rs
Outdated
| @@ -201,7 +214,7 @@ fn check_jwt_rate_limit(state: &SigningState, client_ip: &IpAddr) -> Result<(), | |||
| } | |||
|
|
|||
| /// Checks if a request can successfully authenticate with the JWT secret | |||
| fn check_jwt_auth( | |||
| async fn check_jwt_auth( | |||
There was a problem hiding this comment.
this doesn't need to be async anymore
There was a problem hiding this comment.
Fixed ce562de
| ) -> Result<Response, SignerModuleError> { | ||
| let jwt: Jwt = auth.token().to_string().into(); | ||
|
|
||
| validate_admin_jwt(jwt, &state.admin_secret.read()).map_err(|e| { |
There was a problem hiding this comment.
as a sanity check is the read guard dropped immediately after the function?
There was a problem hiding this comment.
The lock is dropped immediately in case of an Ok() result. In other case, the lock will be kept in the map_err closure, which only logs an error. Would you prefer it to immediately drop in either case?
Co-authored-by: eltitanb <[email protected]> Co-authored-by: ltitanb <[email protected]>
| - `jwt_secrets`: a string with a comma-separated list of `<MODULE_ID>=<JWT_SECRET>` for all modules. | ||
| - `admin_secret`: a string with the secret for the signer admin JWT. | ||
|
|
||
| In the case that someone of those isn't present, that parameter won't be updated. |
There was a problem hiding this comment.
We discussed this out-of-band a bit; the current implementation of reload is going to refresh the JWT secrets and Admin secret from disk if they aren't provided in the call, so it will throw away anything done with the revoke route. If that's not intended then revoke would need some way to write persistent data to disk which isn't currently possible.
Anyway assuming it is intended, then let's just clarify here by replacing this line with the following:
"Parameters that are not provided will not be updated; they will be regenerated using their original on-disk data as though the signer service was being restarted. Note that any changes you made with calls to /revoke_jwt or /reload will be reverted, so make sure you provide any modifications again as part of this call."
Now the
reloadendpoint on the signer module allows to update the JWT secrets too.Also, a new
revoke_moduleendpoint was added, to quickly remove the permissions for a compromised module.This two endpoints are now under a new middleware that validates a special "admin" JWT, whose secret is autogenerated on the
initcommand.